Add PR monitoring: server-driven babysit mode for agent threads - #4428
Add PR monitoring: server-driven babysit mode for agent threads#4428t3dotgg wants to merge 7 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This PR introduces a substantial new feature (PR monitoring/babysit mode) with new server-side services, MCP toolkit, orchestration events, database migration, and UI components spanning ~3000+ lines of new code. New features of this scope warrant human review to validate the design and runtime behavior. You can customize Macroscope's approvability policy. Learn more. |
Threads can now formally monitor a PR after creating it. The agent calls a new monitor_start MCP tool; a dedicated PullRequestMonitor service polls GitHub every 30s, diffs review comments / reviews / check runs against a per-thread cursor, and wakes the thread with a self-contained prompt when there is something to address. The server — never the agent — declares the PR ready from head-SHA-anchored GitHub state. - GitHub monitor snapshot fetch (reviews, threads, check runs, mergeability) with pure cursor-diff and readiness modules - thread.monitor.start/update/end domain events through decider/projector, monitor state on the thread shell, migration 034 (monitor_json) - Wake dispatch protocol: claim → fresh terminal revalidation at the send boundary → dispatch → cursor ack only after accepted start; user turns always win and rewind in-flight wake cursors so events re-diff; 10-wake circuit breaker ends as needs-attention - Monitoring holds the thread unsettled; user settle ends the monitor; session teardown ends it as session-ended - UI: steady cyan Monitoring pill (never masks approval/input/failed), blockers sub-line, header MonitorStrip with Stop, emerald Ready-to-merge state with confirm dialog; clients can only end a monitor as "stopped" Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Stop prepareMcpSession ending the active monitor on session start (session recycles no longer kill their own babysit) - Fail closed at the wake send boundary: a failed fresh PR fetch discards the wake instead of falling back to the stale poll state - Correlate provider.turn.start.failed releases by dispatch time and rewind wakeCount alongside the cursor - Monotonic generation allocator; generation-conditional updateCursor/incrementWake/remove so stale polls can't mutate a newer monitor - Dispatch thread.monitor.end before removing the registration in session teardown; end monitors on thread.archived - Preserve wakeCount on idempotent monitor.start replay (breaker can't be reset) - Key check-run cursors by head SHA + name + run id; detect resolved→unresolved thread reopens - Human changes-requested reviews now block readiness - Thread monitoringStartedAt from the registration so pre-existing unresolved threads don't block readiness forever - Use the PR's base ref in the wake prompt instead of hard-coded main - Boot reconcile: threads projecting "monitoring" with no registration are ended as session-ended Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Ignore review-thread activity whose latest comment is the viewer's own reply, breaking the self-triggered wake cycle - Retry boot reconciliation on the first successful poll instead of swallowing a failed shell snapshot at startup - Remove the registration when archive teardown dispatch fails so archived threads are never polled or woken again - Stamp wake claims with the turn command's createdAt so provider turn-start failures (which echo that timestamp) match the release condition; a fresh post-dispatch timestamp could never match Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Findings from a multi-agent review panel (23 confirmed after two-lens adversarial verification), grouped by root cause: Lifecycle identity - Thread a monitor generation through monitor commands, events, and the projected monitor state; reject updates/ends whose generation is not the active one, and make every registry removal generation-conditional so a delayed lifecycle event cannot delete a newer monitor - Re-validate registration, projected monitor status, archive state, generation, and claim at the wake send boundary (after the fresh fetch, not before it) Event loss - Diff before the readiness exit: actionable activity now wins over ending as ready, so a bot comment landing in the same poll as green CI is never discarded - Persist non-actionable cursor transitions so resolved→reopened is still detectable - Acknowledge the cursor only if the wake claim still exists, so a user-turn release is not overwritten Readiness - Collapse reviews to each reviewer's latest opinionated review, so an approval supersedes that reviewer's earlier changes-requested - Re-include pre-monitor review threads once they are edited or reopened Snapshot integrity - Final terminal/head revalidation after pagination, with one bounded refetch on head change - Fetch the last 10 comments per review thread and track per-thread updatedAt so edits to non-final comments are visible - Reopened threads wake regardless of self-authorship - Include the changes-requested review body in the wake prompt Lifecycle events - Archive atomically emits thread.monitor-ended before thread.archived, mirroring settle - Treat a queued-but-unadopted user turn as busy at the send boundary - Boot reconciliation isolates per-thread failures (Bugbot) UI - Gate ready-state PR actions on the checkout PR matching the monitored PR number - Wake snoozed threads on monitor ready/needs-attention/session-ended - Legacy sidebar renders the ready payoff and a needs-attention state; session failure outranks Monitoring - Reset merge confirmation when PR or monitor status changes - endThreadMonitor honors a caller-provided endedAt Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6d65afe to
0c1de35
Compare
- Serialize wake claim transitions with a semaphore so the post-dispatch cursor ack and a concurrent release can no longer interleave; the ack and wake-count increment are now one critical section with the claim check (Macroscope) - Rewind the cursor on session-set(error): the provider reactor emits it before provider.turn.start.failed, so dropping the claim there left the failure handler nothing to restore and acked events were lost. A clean session finish still just frees the claim (Bugbot, high) - Only mark boot reconciliation complete when every orphan ended successfully, so failures are retried on the next poll (Bugbot) - Drop the registration when session teardown runs before the engine is bound, instead of leaving a monitor that can wake a dead session (Bugbot) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Effect Service Conventions: 2 findings. Two standalone *Shape service interfaces were introduced; the conventions require the service interface to be declared inline in the Context.Service tag and referenced as Service["Service"].
Posted via Macroscope — Effect Service Conventions
| readonly previousCursor: PullRequestMonitorCursor; | ||
| } | ||
|
|
||
| export interface PullRequestSnapshotFetcherShape { |
There was a problem hiding this comment.
Standalone PullRequestSnapshotFetcherShape interface. Same convention as MonitorRegistryShape: declare the interface inline in the Context.Service tag and reference it as PullRequestSnapshotFetcher["Service"] rather than keeping a separate FooShape type.
export class PullRequestSnapshotFetcher extends Context.Service<PullRequestSnapshotFetcher>()(
"t3/monitor/PullRequestMonitor/PullRequestSnapshotFetcher",
{
fetch: (input: { readonly cwd: string; readonly pullRequestNumber: number }) =>
Effect.Effect<PullRequestMonitorSnapshot, GitHubPullRequestMonitorError>,
},
) {}Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Same as the sibling comment: FooShape + Context.Service<Foo, FooShape> is the prevailing pattern in apps/server (43 occurrences, zero using the inline-members overload), so PullRequestSnapshotFetcherShape stays consistent with its neighbors. A codebase-wide migration would be a separate PR.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 355a289. Configure here.
The domain-event fiber can observe session-set(error) after the turn.start command is accepted but before dispatchWake stamps the claim in-flight. Phase-guarding the release left that pending claim behind with nothing else to clear it, blocking every future wake for the thread. Release regardless of phase — releaseClaim only rewinds in-flight claims, so a pending release is just a drop. Regression test verified to fail against the phase-guarded release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Threads can now formally monitor a PR after creating it. The agent calls a new
monitor_startMCP tool; a dedicatedPullRequestMonitorservice polls GitHub every 30s, diffs review comments / reviews / check runs against a per-thread cursor, and wakes the thread with a self-contained prompt when there is something to address. The server — never the agent — declares the PR ready from head-SHA-anchored GitHub state.What's included
gitHubPullRequestMonitor.ts): reviews, review threads + resolution, check runs, head SHA, mergeability; pure cursor-diff and readiness modules with false-green guards (stale approvals don't count, zero checks ≠ green, changes-requested blocks until re-review)thread.monitor-started/-snapshot-updated/-endedthrough decider/projector; monitor state on the thread shell; migration 035 (monitor_json)needs-attentionsession-ended; merged/closed PRs end it asterminalwith the whole poll cycle discarded (no post-merge resurrection)"stopped"— the other reasons are server verdictsDesign docs: UX plan https://lmrxu2kpvpm0.postplan.dev · implementation plan https://nr9llb5mku4z.postplan.dev
Testing
🤖 Generated with Claude Code
Note
High Risk
Large new orchestration + in-process polling path that auto-starts at boot, dispatches agent turns from GitHub state, and uses module-global monitor registrations that are lost on restart until boot reconcile.
Overview
Agents can start PR babysit mode via a new MCP
monitor_starttool (withmonitorcapability on MCP sessions). The server registers the thread in a sharedMonitorRegistry, emitsthread.monitor.*commands/events, and runsPullRequestMonitoron a ~30s poll loop that fetches GitHub PR state, diffs review/CI changes, evaluates merge readiness, and dispatchesthread.turn.startwake prompts when needed—with claim/ack/rewind logic for races, user turns, and provider failures, plus a 10-wakeneeds-attentionstop.Orchestration & persistence: New monitor lifecycle in the decider/projector (start clears settled override; settle/archive end monitoring),
monitor_jsonon projected threads (migration 035), and session teardown that ends monitors before revoking MCP.UI:
MonitorStripin chat (stop / ready / merge confirm) and sidebar status pills (Monitoring, Ready to merge, Needs attention) that do not override approval/input/failed states.Reviewed by Cursor Bugbot for commit d1faaa1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add server-driven PR monitoring with agent babysit mode for orchestration threads
PullRequestMonitoractor starts at server boot, polls GitHub for PR state changes (reviews, check runs, behind-base, thread updates), diffs against a cursor, and wakes the agent thread with a structured prompt when actionable events are detected.MonitorRegistryto track per-thread monitor state (generation, cursor, wake count) module-globally so both the runtime core and MCP routes share the same instance.thread.monitor.start/update/endcommands and corresponding events; the decider and projector handle the full monitor lifecycle and persist state in a newmonitor_jsoncolumn (migration 035).monitor_startthat agents can call to begin monitoring a PR; MCP credentials now include themonitorcapability by default.MonitorStripto the chat header and updates sidebar rows with monitoring-aware pills (Monitoring,Needs attention,Ready to merge) and blockers summary; threads with an active monitor are always considered unsettled.Macroscope summarized d1faaa1.